Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

전남대 BE_안원모 1주차 과제(3단계) #184

Open
wants to merge 17 commits into
base: main
Choose a base branch
from

Conversation

Wonmoan
Copy link

@Wonmoan Wonmoan commented Jun 28, 2024

코드 리뷰 시, 멘토님이 중점적으로 리뷰해줬으면 하는 부분
전체적으로 아직 Spring의 이해도가 낮아서 제대로 수행했는지 모르겠습니다.. 전반적으로 확인해주시면 감사하겠습니다!
그리고 잘 모르다보니 스스로 구현했다기 보다는 구글링에 의존하여 하였는데,
1.제대로 구현한 것인지
2.코드에 불필요한 점은 없는지
궁금합니다.

Copy link

@pkeugine pkeugine left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

안녕하세요 원모님!
앞서 다른 PR 에 남긴 리뷰와 해당 PR 의 리뷰를 한 번 확인해보시고,
이 PR 에 한 번 적용해보시면 좋을 것 같아요.
그런데 step 마다 커밋 기록이 다르던데, 어떻게 커밋하셨던걸까요? 🤔
이왕이면 다음부터는 커밋도 한 번 신경써서 작업해보도록 하죠!
고생하셨고, 앞으로 같이 파이팅해보죠! 🔥 🔥 🔥

@@ -1 +1,32 @@
# spring-gift-product

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

step 1, step 2, 그리고 step 3 가 각각 별도의 커밋 기록을 가지고 있는데 혹시 어떻게 작업해주신건지 궁금하네요...!

@Repository
public class ProductRepository {

private final JdbcTemplate jdbcTemplate;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SimpleJdbcInsert 라는 것도 사용해보는 것은 어떨까요?
어떤 차이가 있는지 확인해보면 좋을 것 같아요.

Comment on lines +20 to +31
private static final class ProductRowMapper implements RowMapper<Product> {

@Override
public Product mapRow(ResultSet rs, int rowNum) throws SQLException {
Product product = new Product();
product.setId(rs.getLong("id"));
product.setName(rs.getString("name"));
product.setPrice(rs.getBigDecimal("price"));
product.setDescription(rs.getString("description"));
return product;
}
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

잘 만들어주셨네요!
해당 로직을 lambda 형식으로도 사용할 수 있는데, 한 번 도전해볼까요?

Comment on lines 32 to 40
// 모든 제품 검색 후 list<Product>로 반환
public List<Product> findAll() {
return jdbcTemplate.query("SELECT * FROM product", new ProductRowMapper());
}

// 주어진 id 가진 제품 검색후 Product 객체로 반환
public Product findById(Long id) {
return jdbcTemplate.queryForObject("SELECT * FROM product WHERE id = ?", new Object[]{id}, new ProductRowMapper());
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

주석 제거해주세요!


// 주어진 id 가진 제품 검색후 Product 객체로 반환
public Product findById(Long id) {
return jdbcTemplate.queryForObject("SELECT * FROM product WHERE id = ?", new Object[]{id}, new ProductRowMapper());

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

해당 queryForObject 메서드는 deprecated 되었어요.
다른 방식을 사용해보면 좋겠네요!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

import org.springframework.jdbc.core.ResultSetExtractor;
import org.springframework.dao.DataAccessException;
// ...

public Product findById(Long id) {
return jdbcTemplate.query(
"SELECT * FROM product WHERE id = ?",
new Object[]{id},
new ResultSetExtractor() {
@OverRide
public Product extractData(ResultSet rs) throws SQLException, DataAccessException {
if (rs.next()) {
Product product = new Product();
product.setId(rs.getLong("id"));
product.setName(rs.getString("name"));
product.setPrice(rs.getBigDecimal("price"));
product.setDescription(rs.getString("description"));
return product;
}
return null;
}
}
);
}
으로 수정 하면 될까요 ?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

괜찮아보이네요!
깃허브에서 사용할 수 있는 기능을 한 번 살펴보고 사용해보시면 좋을 것 같아요.
예를들어 코드는 이렇게 표현해볼 수 있답니다:

public Product findById(Long id) {
    return jdbcTemplate.query("SELECT * FROM product WHERE id = ?",
        new Object[] {id}, new ResultSetExtractor() {
            @Override
            public Product extractData(ResultSet rs)
                throws SQLException, DataAccessException {
                if (rs.next()) {
                    Product product = new Product();
                    product.setId(rs.getLong("id"));
                    product.setName(rs.getString("name"));
                    product.setPrice(rs.getBigDecimal("price"));
                    product.setDescription(rs.getString("description"));
                    return product;
                }
                return null;
            }
        });
}

src/main/java/gift/ProductRepository.java Outdated Show resolved Hide resolved
src/main/java/gift/ProductService.java Outdated Show resolved Hide resolved
Comment on lines 2 to 10
# H2 Database configuration
spring.datasource.url=jdbc:h2:mem:testdb
spring.datasource.driverClassName=org.h2.Driver
spring.datasource.username=sa
spring.datasource.password=password
spring.datasource.platform=h2

# Initialize the database with schema.sql and data.sql
spring.sql.init.mode=always

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

해당 옵션은 어디를 참고하셨나요?
spring.datasource.platform 같은 경우는 deprecated 되었으니 대안을 찾아보면 좋을 것 같아요.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

spring.application.name=spring-gift

H2 Database configuration

spring.datasource.url=jdbc:h2:mem:testdb
spring.datasource.driverClassName=org.h2.Driver
spring.datasource.username=sa
spring.datasource.password=password

Initialize the database with schema.sql and data.sql

spring.sql.init.mode=always
spring.sql.init.schema-locations=classpath:schema.sql
spring.sql.init.data-locations=classpath:data.sql 으로 수정하면 될까요

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

위의 코멘트도 깃허브 포매팅 방식을 익혀서 적용해보면 좋을 것 같네요 ㅋㅋㅋ
네, 좋습니다.
위의 설정들은 혹시 각각 어떤 역할을 하는지 알고 계신가요?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants